Test Linux fallback paths without use of cfg()s#628
Conversation
This PR changes our Linux fallback tests to test the "real" fallback paths rather than using various `cfg()`s within the implementation. As these tests require being run sequentally and only work on Linux, they are configured to require manual invocation. Also renames `tests/mod.rs` to `tests/common.rs` to reuse our tests. Actually testing the netbsd fallback is harder, so that's best left for a future PR. Signed-off-by: Joe Richey <joerichey@google.com>
| path.to_str() == Some("/dev/urandom") | ||
| } | ||
| let mut dir = read_dir("/proc/self/fd").expect("/proc/self exists"); | ||
| assert!(dir.any(|path| is_urandom(path.expect("entry exists")))); |
There was a problem hiding this comment.
Why not simply inline the is_urandom function?
let has_open_urandom = read_dir("/proc/self/fd")
.expect("/proc/self exists")
.any(|dir_entry| {
let path = dir_entry.expect("entry exists").path();
let path = path.canonicalize().expect("entry is valid");
path.to_str() == Some("/dev/urandom")
});
assert!(has_open_urandom);| // Override libc::getrandom to simulate failure | ||
| #[export_name = "getrandom"] | ||
| pub unsafe extern "C" fn fake_getrandom(_: *mut c_void, _: size_t, _: c_uint) -> ssize_t { | ||
| FAKE_GETRANDOM_CALLED.store(true, Ordering::SeqCst); |
There was a problem hiding this comment.
I think AcqRel should be sufficient here?
| - uses: Swatinem/rust-cache@v2 | ||
| - run: cargo test --target=${{ matrix.target }} --features=std | ||
| - run: cargo test --test linux_no_fallback -- --test-threads 1 | ||
| - run: cargo test --test linux_force_fallback -- --test-threads 1 |
There was a problem hiding this comment.
Are you sure that the order of tests is guaranteed?
|
Overall, it's an interesting approach, but I am not sure if it's simpler than the |
|
@josephlr |
I plan to work on this, but it's not the type of thing which should block releases. Sorry for the delay in response. I've recently switched roles inside Google, so things have been a bit chaotic. On the plus side I should now have more time to work on Rust stuff. |
|
This PR is quite stale and has a lot of conflicts with the master branch, so if we are to do it, I think it's better to start from scratch. |
Agreed, thanks for cleaning this up! |
2 participants
This PR changes our Linux fallback tests to test the "real" fallback paths rather than using various
cfg()s within the implementation.As these tests require being run sequentally and only work on Linux, they are configured to require manual invocation.
Also renames
tests/mod.rstotests/common.rsto reuse our tests.Actually testing the netbsd fallback is harder, so that's best left for a future PR.